Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Netkvm: use WinDump for capture and Wireshark for analysis #4097

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

heywji
Copy link
Contributor

@heywji heywji commented Jul 5, 2024

Replace tshark.exe with WinDump.exe for capturing network traffic. This resolves the issue where enabling netkvm driver TxLSO results in no packet length >= 1514 after file transfer, identified as a Wireshark problem.

ID: 2395
Signed-off-by: wji [email protected]

@heywji
Copy link
Contributor Author

heywji commented Jul 5, 2024

Test Result:

 (1/1) Host_RHEL.m9.u5.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.WinNext.x86_64.io-github-autotest-qemu.enable_scatter_windows.q35: PASS (499.27 s)

@heywji
Copy link
Contributor Author

heywji commented Jul 5, 2024

Hi @leidwang and @yanglei-rh,

Could you guys help me review this patch when you are free? Thanks in advance.

@yanglei-rh
Copy link
Contributor

Please re-write the test steps, the whole process is not installed tshark.exe, but you use it to parse the final result, which will cause failure due to the lack of this file. You provide test result is not correct, please reinstall windows guest and test it again, then you will hit I mentioned problem.
In the future, please follow the steps below when providing your test results: clear your env, then apply patch and provide test result. Instead of provide test result based on your develop env.

@heywji
Copy link
Contributor Author

heywji commented Jul 8, 2024

Hi @yanglei-rh,

Thanks for your advice. I will provide the whole test cases which include unattended_install and the case that I want to test.

 (1/2) Host_RHEL.m9.u5.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.WinNext.x86_64.io-github-autotest-qemu.unattended_install.cdrom.extra_cdrom_ks.default_install.aio_threads.q35: STARTED
 (1/2) Host_RHEL.m9.u5.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.WinNext.x86_64.io-github-autotest-qemu.unattended_install.cdrom.extra_cdrom_ks.default_install.aio_threads.q35: PASS (1615.65 s)
 (2/2) Host_RHEL.m9.u5.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.WinNext.x86_64.io-github-autotest-qemu.enable_scatter_windows.q35: STARTED
 (2/2) Host_RHEL.m9.u5.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.WinNext.x86_64.io-github-autotest-qemu.enable_scatter_windows.q35: PASS (502.66 s)
RESULTS    : PASS 2 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
JOB HTML   : /root/avocado/job-results/job-2024-07-08T18.07-70ef483/results.html
JOB TIME   : 2121.74 s

For this patch, I have installed the tshark.exe with the /S parameter with line 20. You know, the Wireshark installer supports the /S parameter for silent installation on Windows.

qemu/tests/cfg/enable_scatter_windows.cfg Outdated Show resolved Hide resolved
x86_64:
wireshark_name = "Wireshark-win64-1.10.1.exe"
i386, i686:
wireshark_name = "Wireshark-win32-1.10.1.exe"
install_wireshark_cmd = "xcopy WIN_UTILS:\${wireshark_name} c:\ /y && c:\${wireshark_name} /S"
installation_cmd = "xcopy WIN_UTILS:\${WinDump_name} c:\ /y && xcopy WIN_UTILS:\${wireshark_name} c:\ /y && c:\${wireshark_name} /S"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems this cmd will copy windump and wireshark to C and then install wireshark, is this expected?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the title of this PR says: "Netkvm: Replace tshark.exe with WinDump.exe".
Why does your code still install wireshark but not windump?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the end of this case, we still need to analyze the packet's result with Wireshark only. However, we have used Windump to capture NetFlow data since Wireshark has a bug.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the WinDump part, WinDump.exe is a portable software that does not need to be installed.

Thank @leidwang and let me think about the patch title again : )

@yanglei-rh
Copy link
Contributor

Hi @yanglei-rh,

Thanks for your advice. I will provide the whole test cases which include unattended_install and the case that I want to test.

 (1/2) Host_RHEL.m9.u5.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.WinNext.x86_64.io-github-autotest-qemu.unattended_install.cdrom.extra_cdrom_ks.default_install.aio_threads.q35: STARTED
 (1/2) Host_RHEL.m9.u5.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.WinNext.x86_64.io-github-autotest-qemu.unattended_install.cdrom.extra_cdrom_ks.default_install.aio_threads.q35: PASS (1615.65 s)
 (2/2) Host_RHEL.m9.u5.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.WinNext.x86_64.io-github-autotest-qemu.enable_scatter_windows.q35: STARTED
 (2/2) Host_RHEL.m9.u5.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.WinNext.x86_64.io-github-autotest-qemu.enable_scatter_windows.q35: PASS (502.66 s)
RESULTS    : PASS 2 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
JOB HTML   : /root/avocado/job-results/job-2024-07-08T18.07-70ef483/results.html
JOB TIME   : 2121.74 s

For this patch, I have installed the tshark.exe with the /S parameter with line 20. You know, the Wireshark installer supports the /S parameter for silent installation on Windows.

But what confuses me is that it always failed with " avocado.core.exceptions.TestError: Failed to parse session log file, status=9009, output='"C:\Program Files\Wireshark\tshark.exe"' is not recognized as an internal or external command," on my side, why you can test pass? Could you please try to test it again with other guest? Such as Win11 or Win2025, or both two.

@heywji
Copy link
Contributor Author

heywji commented Jul 9, 2024

Thanks, guys. Let me do more tests.

@leidwang
Copy link
Contributor

Hi @heywji Please mark this PR as draft status if it is not ready to review,thanks.

@heywji heywji marked this pull request as draft July 24, 2024 04:51
@heywji heywji marked this pull request as ready for review September 14, 2024 04:12
@heywji heywji force-pushed the change_to_tshark branch 2 times, most recently from 1901e8d to 857bfcd Compare September 15, 2024 02:33
@heywji heywji marked this pull request as draft September 24, 2024 03:40
@heywji heywji changed the title Netkvm: replace tshark.exe with WinDump.exe Netkvm: use WinDump for capture and Wireshark for analysis Sep 24, 2024
@heywji heywji marked this pull request as ready for review September 24, 2024 09:21
@heywji
Copy link
Contributor Author

heywji commented Sep 24, 2024

Hi @yanglei-rh @leidwang, Could you guys help review it again since everything works well now?

Test result:

 (1/2) Host_RHEL.m9.u5.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.Win10.x86_64.io-github-autotest-qemu.unattended_install.cdrom.extra_cdrom_ks.default_install.aio_threads.q35: STARTED
 (1/2) Host_RHEL.m9.u5.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.Win10.x86_64.io-github-autotest-qemu.unattended_install.cdrom.extra_cdrom_ks.default_install.aio_threads.q35: PASS (876.53 s)
 (2/2) Host_RHEL.m9.u5.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.Win10.x86_64.io-github-autotest-qemu.enable_scatter_windows.q35: STARTED
 (2/2) Host_RHEL.m9.u5.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.Win10.x86_64.io-github-autotest-qemu.enable_scatter_windows.q35: PASS (401.36 s)

Copy link
Contributor

@yanglei-rh yanglei-rh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@leidwang leidwang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@heywji
Copy link
Contributor Author

heywji commented Oct 9, 2024

Hi @vivianQizhu, Could you help review this as well? Thanks : )

@heywji heywji force-pushed the change_to_tshark branch 3 times, most recently from 5d22a50 to e7ec1d1 Compare October 30, 2024 02:58
@heywji
Copy link
Contributor Author

heywji commented Oct 30, 2024

Test result:

 (1/3) Host_RHEL.m10.u0.qcow2.virtio_scsi.up.virtio_net.Guest.Win10.i386.io-github-autotest-qemu.enable_scatter_windows.q35: STARTED
 (1/3) Host_RHEL.m10.u0.qcow2.virtio_scsi.up.virtio_net.Guest.Win10.i386.io-github-autotest-qemu.enable_scatter_windows.q35:  PASS (391.00 s)
 (2/3) Host_RHEL.m10.u0.qcow2.virtio_scsi.up.virtio_net.Guest.Win10.x86_64.io-github-autotest-qemu.enable_scatter_windows.q35: STARTED
 (2/3) Host_RHEL.m10.u0.qcow2.virtio_scsi.up.virtio_net.Guest.Win10.x86_64.io-github-autotest-qemu.enable_scatter_windows.q35:  PASS (373.25 s)
 (3/3) Host_RHEL.m10.u0.qcow2.virtio_scsi.up.virtio_net.Guest.Win2025.x86_64.io-github-autotest-qemu.enable_scatter_windows.q35: STARTED
 (3/3) Host_RHEL.m10.u0.qcow2.virtio_scsi.up.virtio_net.Guest.Win2025.x86_64.io-github-autotest-qemu.enable_scatter_windows.q35:  PASS (377.38 s)
RESULTS    : PASS 3 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0

qemu/tests/cfg/enable_scatter_windows.cfg Outdated Show resolved Hide resolved
qemu/tests/cfg/enable_scatter_windows.cfg Outdated Show resolved Hide resolved
qemu/tests/enable_scatter_windows.py Outdated Show resolved Hide resolved
Comment on lines 178 to 206
error_context.context("Check if WinDump is installed", test.log.info)
check_result = session.cmd_output(check_windump_installed_cmd)
if windump_name not in check_result:
error_context.context("Install WinDump", test.log.info)
windump_install_cmd = utils_misc.set_winutils_letter(
session, windump_install_cmd
)
status, output = session.cmd_status_output(windump_install_cmd, timeout=timeout)
if status != 0:
test.error(
"Failed to install WinDump, status=%s, output=%s" % (status, output)
)
else:
test.log.info("WinDump is already installed")

error_context.context("Check if wireshark is installed", test.log.info)
check_installed_cmd = params.get("check_installed_cmd")
check_result = session.cmd_output(check_installed_cmd)
check_wireshark_installed_cmd = params.get("check_wireshark_installed_cmd")
check_result = session.cmd_output(check_wireshark_installed_cmd)
if "tshark" not in check_result:
error_context.context("Install wireshark", test.log.info)
install_wireshark_cmd = params.get("install_wireshark_cmd")
install_wireshark_cmd = utils_misc.set_winutils_letter(
session, install_wireshark_cmd
wireshark_installation_cmd = params.get("wireshark_installation_cmd")
wireshark_installation_cmd = utils_misc.set_winutils_letter(
session, wireshark_installation_cmd
)
status, output = session.cmd_status_output(
install_wireshark_cmd, timeout=timeout
wireshark_installation_cmd, timeout=timeout
)
if status:
test.error(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you try to implement them into a for loop? e.g. for tool_name in ("windump", "wireshark") or it could be a dict as you might want to name a different prefix instead of the tool name itself.

@heywji heywji force-pushed the change_to_tshark branch 2 times, most recently from 399d4d3 to 03af606 Compare November 12, 2024 06:44
@heywji heywji force-pushed the change_to_tshark branch 7 times, most recently from 3f7652b to cb94ac9 Compare November 13, 2024 03:42
@heywji
Copy link
Contributor Author

heywji commented Nov 13, 2024

Test Result:

1-Host_RHEL.m9.u6.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.Win11.x86_64.io-github-autotest-qemu.enable_scatter_windows.q35 Finshed 2024-11-12 22:44:24 2024-11-12 22:50:14 PASS 349.327544

@yanglei-rh
Copy link
Contributor

@heywji please re-install a image and clone this patch to test again to verify you patch is correct, since there are include install steps.

Replace tshark.exe with WinDump.exe for capturing network traffic. This
resolves the issue where enabling netkvm driver TxLSO results in no packet
length >= 1514 after file transfer, identified as a Wireshark problem.

Signed-off-by: wji <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants